Skip to content

perf(allocator): add #[cold] annotations to error handling functions#18181

Merged
graphite-app[bot] merged 1 commit intomainfrom
perf/cold-path-annotations
Jan 18, 2026
Merged

perf(allocator): add #[cold] annotations to error handling functions#18181
graphite-app[bot] merged 1 commit intomainfrom
perf/cold-path-annotations

Conversation

@Boshen
Copy link
Copy Markdown
Member

@Boshen Boshen commented Jan 18, 2026

Summary

  • Add #[cold] and #[inline(never)] annotations to error/panic functions in oxc_allocator
  • Improves branch prediction and code layout for hot allocation paths

Functions annotated

File Function Change
bump.rs allocation_size_overflow Added #[cold]
bumpalo_alloc.rs new_layout_err Added #[cold] + #[inline(never)]
bumpalo_alloc.rs handle_alloc_error Added #[cold] + #[inline(never)]
vec2/raw_vec.rs capacity_overflow Added #[cold] + #[inline(never)]

Impact

The #[cold] attribute tells the compiler to:

  1. Assume branches leading to these functions are unlikely
  2. Move cold code away from hot paths (improves i-cache locality)
  3. Avoid inlining cold functions into hot callers

Test plan

  • cargo test -p oxc_allocator passes

🤖 Generated with Claude Code

@Boshen Boshen requested a review from overlookmotel as a code owner January 18, 2026 11:51
Copilot AI review requested due to automatic review settings January 18, 2026 11:51
@github-actions github-actions bot added the C-performance Category - Solution not expected to change functional behavior, only performance label Jan 18, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds performance optimization attributes (#[cold] and #[inline(never)]) to error handling and panic functions in the oxc_allocator crate to improve branch prediction and code layout for hot allocation paths.

Changes:

  • Added #[cold] attribute to allocation_size_overflow in bump.rs (already had #[inline(never)])
  • Added both #[cold] and #[inline(never)] attributes to new_layout_err and handle_alloc_error in bumpalo_alloc.rs
  • Added both #[cold] and #[inline(never)] attributes to capacity_overflow in vec2/raw_vec.rs

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
crates/oxc_allocator/src/bump.rs Added #[cold] to allocation_size_overflow function
crates/oxc_allocator/src/bumpalo_alloc.rs Added #[cold] and #[inline(never)] to new_layout_err and handle_alloc_error functions
crates/oxc_allocator/src/vec2/raw_vec.rs Added #[cold] and #[inline(never)] to capacity_overflow function

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Jan 18, 2026

Merging this PR will not alter performance

✅ 42 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing perf/cold-path-annotations (0dd3702) with main (37482eb)

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jan 18, 2026
Copy link
Copy Markdown
Member Author

Boshen commented Jan 18, 2026

Merge activity

…18181)

## Summary

- Add `#[cold]` and `#[inline(never)]` annotations to error/panic functions in oxc_allocator
- Improves branch prediction and code layout for hot allocation paths

### Functions annotated

| File | Function | Change |
|------|----------|--------|
| `bump.rs` | `allocation_size_overflow` | Added `#[cold]` |
| `bumpalo_alloc.rs` | `new_layout_err` | Added `#[cold]` + `#[inline(never)]` |
| `bumpalo_alloc.rs` | `handle_alloc_error` | Added `#[cold]` + `#[inline(never)]` |
| `vec2/raw_vec.rs` | `capacity_overflow` | Added `#[cold]` + `#[inline(never)]` |

### Impact

The `#[cold]` attribute tells the compiler to:
1. Assume branches leading to these functions are unlikely
2. Move cold code away from hot paths (improves i-cache locality)
3. Avoid inlining cold functions into hot callers

## Test plan

- [x] `cargo test -p oxc_allocator` passes

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app bot force-pushed the perf/cold-path-annotations branch from 0dd3702 to 46cd73d Compare January 18, 2026 12:12
@graphite-app graphite-app bot merged commit 46cd73d into main Jan 18, 2026
22 checks passed
@graphite-app graphite-app bot deleted the perf/cold-path-annotations branch January 18, 2026 12:18
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 18, 2026
overlookmotel added a commit that referenced this pull request Apr 2, 2026
#18168 copied `bumpalo`'s code for `Bump` into `oxc_allocator` crate.
#18172, #18181, and #18234 made a few improvements to the
implementation.

However, #18168 removed various things, and altered others. Notably, it
removed the`MIN_ALIGN` generic param, which we definitely want to keep.

I'm going to be working on the allocator, and would like to start with a
clean slate, beginning with the "known good" implementation of
`bumpalo`.

This PR removes the previous modified `bumpalo` implementation, and
copies latest version of `bumpalo` from main branch into the repo. It
then makes the absolute minimum changes necessary just to make CI pass.

Note: Unlike #18168, this PR keeps all the doctests for `Bump`, using a
trick of adding the crate to it's own `dev-dependencies` with `testing`
feature enabled, and exposing `Bump` only with that feature.

This PR is broken into multiple commits, so we have an exact "trail of
breadcrumbs" of how we've diverged from `bumpalo`. I intend to manually
merge this PR, so that record remains on Github (instead of Graphite
squashing all the commits).

Later PRs in this stack reapply the changes made in #18172, #18181, and
#18234 on top of the fresh base implementation. This PR is a small perf
regression, but that will be won back once those other changes are
reapplied.
graphite-app bot pushed a commit that referenced this pull request Apr 2, 2026
Repeat of #18181.

#20963 wiped all changes to `bumpalo_alloc.rs`, going back to a fresh copy of `bumpalo`. Re-apply changes from #18181 which were lost in the process - marking error handling functions as `#[cold]`.

This PR differs from #18181 in one minor respect. `new_layout_err` is not marked `#[inline(never)]` in this PR. The function is a no-op (unconditionally returns a ZST), so we do want it inlined, but with the`#[cold]`attribute to hint to compiler that it's a cold path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-performance Category - Solution not expected to change functional behavior, only performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants